Repository navigation
Conversation
… walkOnboarding The previous implementation did a single onboardingOverlayLikelyVisible() check before calling walkOnboarding. This created a timing race: if onboarding had not yet rendered at call time the helper returned early, leaving the overlay blocking subsequent steps. walkOnboarding already handles both cases correctly — it polls up to 8 × 400 ms for the overlay before giving up, then no-ops gracefully if not visible. completeOnboardingIfVisible now delegates to it unconditionally, eliminating the race without changing observable behaviour. Also updates the section comment to accurately describe the current 5-step onboarding sequence (Welcome → Local AI → Screen & Accessibility → Tools → Skills) following the removal of MnemonicStep in tinyhumansai#279. Closes part of tinyhumansai#201 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
conversations-web-channel-flow.spec.ts was the only spec that called walkOnboarding directly instead of the shared completeOnboardingIfVisible helper. Align it with all other specs so the single resilient code path is used consistently. Closes tinyhumansai#201 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Updated the e2e Dockerfile to include clang and cmake as additional dependencies, enhancing the build environment for end-to-end testing.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughModified the E2E onboarding helper to unconditionally delegate to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
* fix(e2e): make completeOnboardingIfVisible resilient by delegating to walkOnboarding The previous implementation did a single onboardingOverlayLikelyVisible() check before calling walkOnboarding. This created a timing race: if onboarding had not yet rendered at call time the helper returned early, leaving the overlay blocking subsequent steps. walkOnboarding already handles both cases correctly — it polls up to 8 × 400 ms for the overlay before giving up, then no-ops gracefully if not visible. completeOnboardingIfVisible now delegates to it unconditionally, eliminating the race without changing observable behaviour. Also updates the section comment to accurately describe the current 5-step onboarding sequence (Welcome → Local AI → Screen & Accessibility → Tools → Skills) following the removal of MnemonicStep in tinyhumansai#279. Closes part of tinyhumansai#201 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(e2e): use completeOnboardingIfVisible in conversations spec conversations-web-channel-flow.spec.ts was the only spec that called walkOnboarding directly instead of the shared completeOnboardingIfVisible helper. Align it with all other specs so the single resilient code path is used consistently. Closes tinyhumansai#201 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(e2e): add clang and cmake to Dockerfile dependencies Updated the e2e Dockerfile to include clang and cmake as additional dependencies, enhancing the build environment for end-to-end testing. --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…2e): make completeOnboardingIfVisible resilient by delegating to walkOnboarding\n\nThe previous implementation did a single onboardingOverlayLikelyVisible()\ncheck before calling walkOnboarding. This created a timing race: if onboarding\nhad not yet rendered at call time the helper returned early, leaving the\noverlay blocking subsequent steps.\n\nwalkOnboarding already handles both cases correctly — it polls up to\n8 × 400 ms for the overlay before giving up, then no-ops gracefully if not\nvisible. completeOnboardingIfVisible now delegates to it unconditionally,\neliminating the race without changing observable behaviour.\n\nAlso updates the section comment to accurately describe the current 5-step\nonboarding sequence (Welcome → Local AI → Screen & Accessibility → Tools →\nSkills) following the removal of MnemonicStep in tinyhumansai#279.\n\nCloses part of tinyhumansai#201\n\n\n* fix(e2e): use completeOnboardingIfVisible in conversations spec\n\nconversations-web-channel-flow.spec.ts was the only spec that called\nwalkOnboarding directly instead of the shared completeOnboardingIfVisible\nhelper. Align it with all other specs so the single resilient code path\nis used consistently.\n\nCloses tinyhumansai#201\n\n\n* fix(e2e): add clang and cmake to Dockerfile dependencies\n\nUpdated the e2e Dockerfile to include clang and cmake as additional dependencies, enhancing the build environment for end-to-end testing.\n\n---------\n\n
Summary
completeOnboardingIfVisiblenow delegates unconditionally towalkOnboardinginstead of gating on a pre-check that could fire before the overlay rendered.walkOnboardingalready polls up to 8 × 400 ms for the overlay, so the extra guard was redundant and introduced a race window.conversations-web-channel-flowspec updated to usecompleteOnboardingIfVisible(matching the rest of the suite and removing the directwalkOnboardingcall).e2e/Dockerfilegainsclang libclang-dev cmake, required bywhisper-rs-sys(bindgen + CMake build of whisper.cpp) on the Linux E2E runner.Problem
completeOnboardingIfVisiblecalledonboardingOverlayLikelyVisible()synchronously before the overlay had time to render after auth. If the check ran too early it returnedfalseand the helper silently skipped onboarding, leaving the app stuck behind the overlay for the rest of the spec.walkOnboardingdirectly rather than the shared helper, diverging from the pattern the rest of the suite uses.clang,libclang-dev, andcmake, causingwhisper-rs-systo fail at the bindgen and CMake stages duringtest:e2e:build.Solution
if (await onboardingOverlayLikelyVisible())guard fromcompleteOnboardingIfVisible; the function now callswalkOnboardingdirectly.walkOnboarding's internal polling loop handles the timing safely — it waits for the overlay and exits cleanly when it never appears (already on Home), so no extra guard is needed.completeOnboardingIfVisible.clang libclang-dev cmaketo theapt-get installblock ine2e/Dockerfileso the image can buildwhisper-rs-syscleanly on both x86-64 and aarch64 Linux.Submission Checklist
completeOnboardingIfVisibleto describe delegation and polling behaviour.Impact
completeOnboardingIfVisibleafter auth.clang libclang-dev cmake).Related
Summary by CodeRabbit
Release Notes
This release contains internal testing and infrastructure improvements with no user-facing changes.
Tests
Chores